Fix #1473: warn when fclose() is used as a while loop condition#8444
Fix #1473: warn when fclose() is used as a while loop condition#8444francois-berder wants to merge 7 commits into
Conversation
|
francois-berder
force-pushed
the
pr-1473
branch
from
3491db9 to
28f2247
Compare
| { | ||
| reportError(tok, Severity::warning, | ||
| "fcloseInLoopCondition", | ||
| "fclose() should not be used as a loop condition.\n" |
There was a problem hiding this comment.
could there be some safe usage where fclose is used in the loop condition by intention:
while (fclose(f)) {
..dostuff..
f = fopen(..);
}
it would be interesting if you would run test-my-pr.py and see if we generate any warnings..
There was a problem hiding this comment.
let's consider how to make the short message a bit more specific about what problems there are.
There was a problem hiding this comment.
I spent a bit of time looking for the right message. In the end, I concluded that we should nearly always warn if fclose is used in a while loop condition simply because it is a code smell. This is why, I also set the severity of the message to warning to better reflect the fact that it may not always cause issues.
There was a problem hiding this comment.
it would be interesting if you would run test-my-pr.py and see if we generate any warnings..
Good point, I will try that but it takes a little while to run. By the way, I spent a bit of time improving the various scripts (daca2-download.py, daca2-getpackages.py and test-my-pr.py) but it is not yet ready.
There was a problem hiding this comment.
@danmar I updated the error message and ran test-my-pr.py. The log is not that interesting, there is still a few diff in:
/home/francois/daca2-packages/c-munipack_2.1.39.orig.tar.xz
libraries:posix,gnu,bsd,gtk,python
diff:
main C-Munipack-2.1.39-source/thirdparty/cfitsio/fits_hdecompress.c:735:13: style: Condition 'dmin<dmax' is always false [knownConditionTrueFalse]
C-Munipack-2.1.39-source/thirdparty/cfitsio/fits_hdecompress.c:730:9: note: dmin is assigned '(((((hp-h0)>(h0-hm))?(hp-h0):(h0-hm))<0)?(((hp-h0)>(h0-hm))?(hp-h0):(h0-hm)):0)<<2' h
ere.
C-Munipack-2.1.39-source/thirdparty/cfitsio/fits_hdecompress.c:729:9: note: dmax is assigned '(((((hp-h0)<(h0-hm))?(hp-h0):(h0-hm))>0)?(((hp-h0)<(h0-hm))?(hp-h0):(h0-hm)):0)<<2' h
ere.
C-Munipack-2.1.39-source/thirdparty/cfitsio/fits_hdecompress.c:735:13: note: Condition 'dmin<dmax' is always false
…oop condition Using fclose() as a while loop condition closes the file on every iteration and operates on an already-closed file handle from the second iteration onward. Signed-off-by: Francois Berder <fberder@outlook.fr>
…while loop condition
…d as a while loop condition
… is used as a while loop condition
…close() is used as a while loop condition
… when fclose() is used as a while loop condition
… warn when fclose() is used as a while loop condition
2 participants
Using fclose() as a while loop condition closes the file on every iteration and operates on an already-closed file handle from the second iteration onward.